Skip to content

Build: Write trace files on failing stories tests #33510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
ndelangen wants to merge 2 commits into
base: next
Choose a base branch
from
Draft

Build: Write trace files on failing stories tests #33510

ndelangen wants to merge 2 commits into from

Conversation

@ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 12, 2026 •
edited by coderabbitai bot
Loading

Closes: #32686

What I did

Add the saving of trace files for failing (stories) tests

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Tests
    • Updated testing infrastructure configuration to enhance failure diagnostics in the Storybook testing environment.

✏️ Tip: You can customize this high-level summary in your review settings.

@ndelangen ndelangen self-assigned this Jan 12, 2026
@ndelangen ndelangen added build Internal-facing build tooling & test updates ci:normal portable stories labels Jan 12, 2026
@ndelangen ndelangen changed the title turn on trace Build: Write trace files on failing stories tests Jan 12, 2026
@ndelangen ndelangen requested a review from yannbf January 12, 2026 10:08
@nx-cloud
Copy link

nx-cloud bot commented Jan 12, 2026 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 8c3f48f

Command Status Duration Result
nx run-many -t compile,check,knip,test,pretty-d... ❌ Failed 8m 57s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-12 11:42:29 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

📝 Walkthrough

Walkthrough

A new Playwright trace configuration option is added to the Vitest Storybook configuration file, enabling trace retention on test failures without modifying other existing settings.

Changes

Cohort / File(s) Summary
Vitest Storybook Configuration
code/vitest.config.storybook.ts		 Added trace: 'retain-on-failure' option to the Playwright browser provider settings for enhanced test failure diagnostics.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39b8e53 and e606452.

📒 Files selected for processing (1)
  • code/vitest.config.storybook.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with yarn prettier --write <file>

Files:

  • code/vitest.config.storybook.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run ESLint checks using yarn lint:js:cmd <file> or the full command cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives to fix linting errors before committing

Files:

  • code/vitest.config.storybook.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode across all packages

Files:

  • code/vitest.config.storybook.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/vitest.config.storybook.ts
🧠 Learnings (9)
📓 Common learnings 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:31.838Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
📚 Learning: 2025-11-24T17:49:59.279Z 
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests

Applied to files:

  • code/vitest.config.storybook.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: normal
  • GitHub Check: nx
  • GitHub Check: nx
  • GitHub Check: nx
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/vitest.config.storybook.ts (1)

44-44: LGTM! Trace configuration enables helpful debugging for failing tests.

The trace: 'retain-on-failure' setting appropriately captures Playwright traces only when tests fail, which aligns well with the objective of debugging flaky tests without accumulating unnecessary trace files. The placement at the browser configuration level is correct and the value is supported by Vitest's browser mode with the Playwright provider.

Comment @coderabbitai help to get the list of available commands and usage tips.

@ndelangen ndelangen marked this pull request as draft January 13, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yannbf yannbf Awaiting requested review from yannbf

At least 1 approving review is required to merge this pull request.

Assignees

@ndelangen ndelangen

Labels

build Internal-facing build tooling & test updates ci:normal portable stories

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Build]: Upgrade codebase to use Vitest to v4 and test traces

2 participants

@ndelangen